-
-
Couldn't load subscription status.
- Fork 4
Some cleanups to the codebase #293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Tests have been moved in the corresponding internal package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review @per1234! Are we using this package somewhere? I could not find any reference to it. If it's not used I'd prefer to remove it (even if it's technically a breaking change, I'm pretty sure nobody outside Arduino is using it). If we are using I'd like to replace it with a package in the |
Yes, it is used here: |
It doesn't make sense to move it to the |
I see. What about moving I think this would give us some long-term advantages:
|
Sounds good. |
2484717 to
4a522c5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Don't forget to open a PR to change this workflow: https://github.com/arduino/library-registry/blob/production/.github/workflows/assets/validate-registry/main.go
As pointed out by @per1234
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Cristian!
io/ioutilswith the equivalent callslibrariespackage at the top level, and merged the tests with the existinginternal/librariespackage.